Skip to content

fix(#999): provide TARGET_REPO_DIR to validation loop for Signed-off-by check - #1050

Merged
ggallen merged 2 commits into
mainfrom
agent/999-fix-validation-target-repo-dir
Aug 27, 2026
Merged

fix(#999): provide TARGET_REPO_DIR to validation loop for Signed-off-by check#1050
ggallen merged 2 commits into
mainfrom
agent/999-fix-validation-target-repo-dir

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Add TARGET_REPO_DIR: "${REPO_DIR}" to env.runner in harness/fix.yaml and harness/code.yaml
  • This enables the validate-code-output.sh Part 2 checks (Signed-off-by trailer detection and pre-commit gate) during the in-sandbox validation loop, so agent-fixable errors trigger a retry iteration instead of reaching the post-script hard gate

Problem

The validate-code-output.sh script guards Part 2 behind a TARGET_REPO_DIR conditional — when the variable is unset, the script soft-passes after schema validation and never checks for Signed-off-by trailers. Neither harness provided TARGET_REPO_DIR in env.runner, so the validation loop only ran schema validation (Part 1). Signed-off-by trailers passed in-sandbox validation and were caught only by the post-script, which terminally rejected the push and required human re-dispatch.

Testing

  • All 24 validate-code-output-test.sh tests pass (including signoff-consumes-iteration which exercises the exact Signed-off-by detection path)
  • YAML validation confirms TARGET_REPO_DIR is present in both harness configs
  • lint-agent-docs hook passes

Closes #999

Post-script verification

  • Branch is not main/master (agent/999-fix-validation-target-repo-dir)
  • Secret scan passed (gitleaks — 2fbb7da338097ad4c3d659820444c7523a0f1e63..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 26, 2026 17:43
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:47 PM UTC · Completed 5:58 PM UTC

Commit: c4ebcc4 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.32

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 26, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Tiny additive config change (3 lines across 3 files) by a known bot, exactly matching issue scope. Protected-path flag elevates T1 slightly (1.5). T2 is elevated (3.43) because all three files are high-churn with many fix-related commits and multiple distinct authors in the last 90 days, but the change itself is minimal and easily revertible. T3 is low (1.17) with clear issue alignment. Anchored to prior score of 2. Overall moderate risk.

Previous run

Risk Assessment: moderate (2/5)

Details

Tiny additive config change (2 lines across 2 files) by a known bot, exactly matching issue scope. Protected-path flag elevates T1 slightly. T2 is elevated because harness/code.yaml and harness/fix.yaml are high-churn files with 11 commits in 30 days across 10 distinct authors and multiple recent fixes, but the change itself is minimal and easily revertible. Overall moderate risk.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] harness/code.yaml, harness/fix.yaml — Both files are under the harness/ protected path prefix. The PR links to issue Fix agent in-sandbox validation should catch Signed-off-by trailers before post-script #999 and explains the rationale for the change (enabling the Signed-off-by validation gate that was silently skipped due to a missing TARGET_REPO_DIR env var). Human approval is always required for protected-path changes, regardless of context.
Previous run

Review

Findings

Medium

  • [protected-path] harness/code.yaml, harness/fix.yaml — Both files are under the harness/ protected path prefix. The PR links to issue Fix agent in-sandbox validation should catch Signed-off-by trailers before post-script #999 and explains the rationale for the change (enabling the Signed-off-by validation gate that was silently skipped due to a missing TARGET_REPO_DIR env var). Human approval is always required for protected-path changes, regardless of context.
Previous run (2)

Review

Findings

Medium

  • [protected-path] harness/code.yaml, harness/fix.yaml — Both files are under the harness/ protected path prefix. The PR links to issue Fix agent in-sandbox validation should catch Signed-off-by trailers before post-script #999 and explains the rationale for the change (enabling the Signed-off-by validation gate that was silently skipped due to a missing TARGET_REPO_DIR env var). Human approval is always required for protected-path changes, regardless of context.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 26, 2026
@ggallen ggallen removed the requires-manual-review Review requires human judgment label Aug 26, 2026
@ggallen

ggallen commented Aug 26, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@ggallen

ggallen commented Aug 26, 2026

Copy link
Copy Markdown
Member

/fs-fix

The functional tests fail because run-fullsend.sh does not set REPO_DIR in the eval environment. The new TARGET_REPO_DIR: "${REPO_DIR}" references in harness/code.yaml and harness/fix.yaml cause fullsend's env validation to reject the run immediately:

  env.runner[TARGET_REPO_DIR]: host variable REPO_DIR is not set (referenced in "${REPO_DIR}")

Fix: in eval/scripts/run-fullsend.sh, inside the code|fix) case block (around line 139), add:

  emit_env "REPO_DIR" "${TARGET_DIR}"

This mirrors production where REPO_DIR=${GITHUB_WORKSPACE}/target-repo. In the eval harness, TARGET_DIR is already set to ${EVAL_GH_WORKSPACE}/target-repo, so the value is equivalent.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:30 PM UTC · Completed 8:36 PM UTC

Commit: c4ebcc4 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.29

fullsend-ai-coder Bot added a commit that referenced this pull request Aug 26, 2026
Add `emit_env "REPO_DIR" "${TARGET_DIR}"` to the code|fix case block
so that harness env validation accepts the TARGET_REPO_DIR: "${REPO_DIR}"
references added in harness/code.yaml and harness/fix.yaml. Mirrors
production where REPO_DIR=${GITHUB_WORKSPACE}/target-repo.

Addresses review feedback on #1050
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Added emit_env REPO_DIR to the code|fix case block in eval/scripts/run-fullsend.sh, fixing fullsend env validation failure caused by unset REPO_DIR referenced by TARGET_REPO_DIR in harness YAML files.

Fixed (1):

  1. emit REPO_DIR in run-fullsend.sh for code/fix eval runs (eval/scripts/run-fullsend.sh): Added emit_env "REPO_DIR" "${TARGET_DIR}" to the code|fix case block in run-fullsend.sh so that the TARGET_REPO_DIR: "${REPO_DIR}" references in harness/code.yaml and harness/fix.yaml resolve correctly during eval runs, mirroring production where REPO_DIR=${GITHUB_WORKSPACE}/target-repo

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:38 PM UTC · Completed 8:53 PM UTC

Commit: 890b840 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.37

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 26, 2026
…by check

The validate-code-output.sh script guards its Part 2 (Signed-off-by
trailer check and pre-commit gate) behind a TARGET_REPO_DIR
conditional. When TARGET_REPO_DIR is not set, Part 2 is silently
skipped and only schema validation runs. Neither the fix nor code
harness provided TARGET_REPO_DIR in their env.runner blocks, so the
validation loop never caught Signed-off-by trailers — they passed
in-sandbox validation and were only caught by the post-script hard
gate, forcing unnecessary human re-dispatch.

Add TARGET_REPO_DIR: "${REPO_DIR}" to env.runner in both
harness/fix.yaml and harness/code.yaml so the validation loop script
receives the path to the target repo and runs the full Part 2 check
(Signed-off-by + pre-commit gate) during in-sandbox retry iterations.

Closes #999
Add `emit_env "REPO_DIR" "${TARGET_DIR}"` to the code|fix case block
so that harness env validation accepts the TARGET_REPO_DIR: "${REPO_DIR}"
references added in harness/code.yaml and harness/fix.yaml. Mirrors
production where REPO_DIR=${GITHUB_WORKSPACE}/target-repo.

Addresses review feedback on #1050
@ggallen
ggallen force-pushed the agent/999-fix-validation-target-repo-dir branch from 890b840 to 580410e Compare August 27, 2026 00:43
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 27, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:45 AM UTC · Completed 12:59 AM UTC

Commit: 580410e · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.58

@fullsend-ai-review fullsend-ai-review Bot removed the risk/moderate PR risk: moderate label Aug 27, 2026
@ggallen
ggallen added this pull request to the merge queue Aug 27, 2026
Merged via the queue into main with commit 4b073c3 Aug 27, 2026
39 checks passed
@ggallen
ggallen deleted the agent/999-fix-validation-target-repo-dir branch August 27, 2026 01:18
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 27, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:19 AM UTC · Completed 1:29 AM UTC

Commit: 580410e · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.21

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1050 — provide TARGET_REPO_DIR to validation loop

Timeline

Issue #999 (created by the retro agent) identified that the fix/code agent validation loop silently skipped Part 2 of validate-code-output.sh (Signed-off-by trailer detection) because TARGET_REPO_DIR was not set in env.runner. The triage agent (run 32993127879, $1.12) confirmed the root cause. The code agent (run 32994587408, $4.13) opened PR #1050 adding TARGET_REPO_DIR: "${REPO_DIR}" to harness/code.yaml and harness/fix.yaml. However, the code agent missed that REPO_DIR is not emitted in the eval environment — eval/scripts/run-fullsend.sh did not include an emit_env "REPO_DIR" call for code/fix agents. Functional tests failed immediately. The review agent (run 32995841257, $3.32) flagged only the protected-path requirement and did not catch the eval gap or note the CI failures. After ~3 hours, ggallen diagnosed the issue and triggered /fs-fix with explicit instructions. The fix agent (run 33010586691, $1.29) added the one-line emit_env fix. A second review ran on the fix push ($3.37), and a third redundant review ($3.58) was triggered when ggallen rebased the branch onto newer main before merging. Total agent cost: ~$16.81.

What went well

  • The triage agent correctly identified the root cause on the first attempt.
  • The fix agent executed ggallen's instructions precisely and efficiently ($1.29, 6 minutes).
  • The overall fix was correct and all tests passed after the fix agent's iteration.

What could be improved

Code agent missed the eval script dependency. The code agent read eval/scripts/run-fullsend.sh twice and saw the emit_env calls for other variables in the code|fix) case block, but did not notice that REPO_DIR was absent from that list — even though its own change introduced a new ${REPO_DIR} dependency. This caused a rework cycle costing ~$4.66 in agent spend plus ~3 hours of wall time.

Evidence for existing open issues

Proposals filed

guyoron1 pushed a commit to guyoron1/agents that referenced this pull request Aug 27, 2026
… REPO_DIR

Production workflows set TARGET_REPO_DIR via setup-agent-env.sh (stripping
the FIX_/CODE_ prefix). The harness YAML files introduced in fullsend-ai#1050
referenced ${REPO_DIR}, which is never set in production — only in the eval
script. Change the harness to pass-through ${TARGET_REPO_DIR} and update
the eval script to emit TARGET_REPO_DIR to match.

Closes fullsend-ai#1062

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix agent in-sandbox validation should catch Signed-off-by trailers before post-script

1 participant